Skip to content

Integrate harmonic mode selection and waveform plugin support - #5381

Open
WuShichao wants to merge 3 commits into
gwastro:masterfrom
WuShichao:feature/pycbc-waveform-mode-integration
Open

Integrate harmonic mode selection and waveform plugin support#5381
WuShichao wants to merge 3 commits into
gwastro:masterfrom
WuShichao:feature/pycbc-waveform-mode-integration

Conversation

@WuShichao

Copy link
Copy Markdown
Member

Allow mode_array entries to carry an eccentric harmonic index n as a (l, m, n) 3-tuple alongside the usual (l, m) pairs, guarding non- eccentric LAL-based approximants against it with a clear error instead of a confusing unpack failure.

Add add_custom_waveform_modes() and the pycbc.waveform.fd_modes / td_modes plugin entry-point groups, so external packages can register mode-by-mode (rather than only summed) FD/TD waveform generators through get_fd_waveform_modes/get_td_waveform_modes.

Standard information about the request

This is a: bug fix, new feature, efficiency update, other (please describe)

This change affects: the offline search, the live search, inference, PyGRB

This change changes: documentation, result presentation / plotting, scientific output

This change: has appropriate unit tests, follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines

This change will: break current functionality, require additional dependencies, require a new release, other (please describe)

Motivation

We will use pyEFPEHM through the PyCBC waveform plugin, but we need to get each (l,m,n) mode from the eccentric waveform, then we can apply the LISA single link response to it, and then TDI.

Contents

Links to any issues or associated PRs

Testing performed

Additional notes

  • The author of this pull request confirms they will adhere to the code of conduct

Allow mode_array entries to carry an eccentric harmonic index n as a
(l, m, n) 3-tuple alongside the usual (l, m) pairs, guarding non-
eccentric LAL-based approximants against it with a clear error instead
of a confusing unpack failure.

Add add_custom_waveform_modes() and the pycbc.waveform.fd_modes /
td_modes plugin entry-point groups, so external packages can register
mode-by-mode (rather than only summed) FD/TD waveform generators
through get_fd_waveform_modes/get_td_waveform_modes.
@WuShichao
WuShichao requested a review from ahnitz July 23, 2026 14:07
@ahnitz
ahnitz requested a review from cdcapano July 23, 2026 15:36
@ahnitz

ahnitz commented Jul 23, 2026

Copy link
Copy Markdown
Member

Adding @cdcapano as he may have some further comments.

Overall, I think this makes sense as a first PR. It doesn't seem to restrict much. That has the downside of not really knowing what to do, but conventions will probably have to come later to try to synchronize waveform formats so that we can actually do something with them.

@WuShichao WuShichao added the LISA label Jul 23, 2026
Comment thread pycbc/waveform/__init__.py Fixed
Comment thread pycbc/waveform/plugin.py Fixed
Targeted fixes for the two issues qlty attributed specifically to this
PR's diff (not pre-existing code):

- pycbc/waveform/__init__.py: add_custom_waveform_modes was imported
  but unused (F401); mark it as an explicit re-export
  (`as add_custom_waveform_modes`), matching the existing convention
  for this file's other re-exported names, without reordering the
  surrounding (pre-existing, never-isorted) import block -- doing so
  was tried and reverted, since it turns lines that were previously
  untouched by this branch into "changed" lines from git's point of
  view, which surfaces several unrelated pre-existing F401/F403
  findings in that block as if newly introduced by this PR.
- pycbc/waveform/plugin.py: sort the one import line inside the new
  add_custom_waveform_modes() function (I001).
- pycbc/waveform/waveform.py, pycbc/waveform/waveform_modes.py: apply
  ruff's quote/wrapping conventions only to the lines this PR actually
  added (mode_array validation in _check_lal_pars,
  get_hm_length_in_time, and get_imrphenomxh_modes).

Not fixed (left for a separate, maintainer-scoped decision): qlty
also reports these 3 files, plus __init__.py's whole import block, as
"unformatted"/"unsorted". Reproduced locally: running `ruff format`
on waveform.py alone changes 1000+ lines end to end (quote style,
import wrapping, blank lines) starting from line 39, i.e. the file has
never been run through ruff format -- none of that is attributable to
this PR's actual ~20-line diff. Reformatting whole files as a side
effect of this PR seemed like the wrong call to make unilaterally, so
qlty_check will still fail on those file-level findings.
@WuShichao
WuShichao force-pushed the feature/pycbc-waveform-mode-integration branch from 8867826 to 3a09d95 Compare July 28, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants